Skip to content

fix(desktop): preserve Windows PATH for ACP probes#2533

Closed
aleixrodriala wants to merge 7 commits into
block:mainfrom
aleixrodriala:fix/windows-acp-probe-path
Closed

fix(desktop): preserve Windows PATH for ACP probes#2533
aleixrodriala wants to merge 7 commits into
block:mainfrom
aleixrodriala:fix/windows-acp-probe-path

Conversation

@aleixrodriala

@aleixrodriala aleixrodriala commented Jul 23, 2026

Copy link
Copy Markdown

Summary

  • preserve the native process PATH when login-shell PATH discovery is unavailable
  • include the same existing well-known Windows Node/npm directories used by the install-shell fix
  • add regression tests for process-PATH fallback and login-shell precedence

This is stacked on #2247 and should be reviewed as the single additional commit 06e8a5e9. Once #2247 lands, the effective diff should collapse to desktop/src-tauri/src/managed_agents/runtime/path.rs.

Root cause

#2247 fixes PATH composition for the ACP install subprocess, but runtime discovery and launched agents use managed_agents/runtime/path.rs::build_augmented_path() instead. On Windows, login_shell_path() is intentionally None, managed Node is unsupported, and the existing builder did not preserve the native process PATH or add well-known Node directories.

As a result, a correctly installed npm shim such as %APPDATA%\Buzz\node-tools\codex-acp.cmd starts but cannot resolve node.exe. Buzz then classifies the adapter as outdated and retries installation, leaving onboarding at Installation failed.

User impact

The affected Windows v0.4.23 machine had Node and Codex installed. Installing @agentclientprotocol/codex-acp 1.1.7 into Buzz's private prefix was not sufficient: the live onboarding UI still failed. Reproducing Buzz's exact augmented PATH produced "node" is not recognized as an internal or external command.

A temporary local Node delegate placed in Buzz's managed prefix made the exact probe succeed and clicking Retry changed the live UI to Ready. That delegate is only a local workaround and is not part of this PR; this source change fixes the underlying PATH construction.

Validation

  • affected Windows machine: failed probe reproduced with Buzz's current PATH; equivalent fixed PATH returned @agentclientprotocol/codex-acp 1.1.7; onboarding confirmed Ready
  • focused runtime/path.rs tests: 7 passed, 0 failed
  • rustfmt --edition 2021 desktop/src-tauri/src/managed_agents/runtime/path.rs
  • git diff --check

The full Tauri crate test was not completed in this WSL environment because its Linux build requires the full WebKitGTK development stack.

tzarebczan and others added 7 commits July 22, 2026 13:21
@tzarebczan
fix(desktop): Windows runtime install discovery and quiet consoles
be6ec4b 
Goose install could report success while discovery still saw NotInstalled
(binary in %USERPROFILE%\goose off PATH), blocking onboarding Next (block#2239).
Install shells also dropped the process PATH on Windows so system npm was
invisible for Codex/Claude adapters (block#2238). Console-subsystem probes
stole focus via Git Bash.

- Probe well-known Windows Node/npm/goose paths; fall back to process PATH
  for install shells; pin goose GOOSE_BIN_DIR to ~/.local/bin on Windows.
- Post-install resolve check; SetupStep only shows Installed after discovery.
- Centralize CREATE_NO_WINDOW via windows_console::hide_console (and
  buzz-dev-mcp hide helper) on spawn sites this fix touches.

Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@tzarebczan
fix(desktop): drop block#2241 revert; Windows-safe vite beforeDev; CH…
58ef77e 
…ECK AGAIN

- Restore Justfile + CONTRIBUTING.md from main (accidental soft-reset
  squash had undone Hermit-pinned lefthook from block#2241).
- beforeDevCommand: use `pnpm exec vite` instead of Unix-only `exec`
  (tauri.conf + instance-env) so Windows cmd can start the dev server.
- After install success, reuse the existing CHECK AGAIN / CHECKING…
  pattern so rediscovery is not a permanent spinner.

Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@tzarebczan
fix(buzz-acp): CREATE_NO_WINDOW for agent adapter spawn on Windows
b86c595 
Desktop launches buzz-acp windowless; without the same flag the adapter
child (cmd.exe/node, buzz-agent, …) allocates its own console per agent.

Refs: block#2292, PR block#2247 review
Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@tzarebczan
fix(windows): prefer Git Bash over WindowsApps bash alias
924778e 
PATH often lists %LOCALAPPDATA%\Microsoft\WindowsApps\bash.exe first;
that App Execution Alias launches WSL and can hang lookups (~15s+) while
real Git Bash is sub-second. Prefer git-derived / install / registry
paths, then PATH bash, and skip WindowsApps (and System32) on PATH.

Refs: block#2328, PR block#2247
Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@tzarebczan
fix(windows): CREATE_NO_WINDOW for MCP and system rg children
259a28a 
Nested CreateProcess sites still flash consoles when the parent is
windowless: buzz-agent MCP spawns and buzz-dev-mcp delegated rg.

Refs: PR block#2247 review, block#2292
Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@tzarebczan
refactor(windows): consolidate no-window helpers and bash skip logic
c51b985 
- Route desktop spawns through windows_console::hide_console (runtime,
  taskkill); stop re-inlining CREATE_NO_WINDOW.
- Add buzz-dev-mcp windows_console for std/tokio; use from shell + rg.
- Single hide_console helper in buzz-acp and buzz-agent MCP spawn.
- Drop redundant WindowsApps also_skip; one path_looks_like check.
- Align Doctor/MCP PATH scan API + KEEP IN SYNC notes (full shared
  resolver deferred: buzz-agent forbids unsafe registry code).
- SetupStep: one CheckAgainButton for auth-unknown and post-install.

No push.

Signed-off-by: Thomas Zarebczan <thomas.zarebczan@gmail.com>
@aleixrodriala
fix(desktop): preserve Windows PATH for ACP probes
06e8a5e 
Signed-off-by: Aleix <aleixrodriala@gmail.com>
@aleixrodriala aleixrodriala mentioned this pull request Jul 23, 2026
Closed
5 tasks
@wpfleger96 wpfleger96 mentioned this pull request Jul 23, 2026
Merged
@aleixrodriala
aleixrodriala marked this pull request as ready for review July 23, 2026 21:30
@aleixrodriala
aleixrodriala requested a review from a team as a code owner July 23, 2026 21:30
@wpfleger96

wpfleger96 commented Jul 26, 2026

Copy link
Copy Markdown
Member

🤖 hey @aleixrodriala — thank you for this contribution. It covers a lot of ground: four crates, the Git Bash probe, nvm layouts, console-window suppression. Some of what it fixes is now on main and some of it we're still missing, so let me be specific about why I'm closing it.

The PATH problem it set out to fix is now handled on main by three merged commits: #2563 (cca16635d), #2753 (3bd3a014c), and #2750 (f3981dbfe), the last covering native PowerShell install spawning. Beyond that, the PR is 25 files across four crates with several unrelated concerns in one diff, and it's conflicting against main. Rebasing it is more work than re-landing the pieces that are still missing, and each of those would be easier to review on its own.

Two things in here that I checked and are genuinely still gaps, if you want to pick either up as a focused PR:

  1. nvm-windows discovery — the one I'd most like to see. find_nvm_default_bin (managed_agents/discovery.rs) only understands POSIX nvm layout (~/.nvm/versions/node); NVM_SYMLINK and NVM_HOME appear nowhere in the tree, and the Windows arm of common_binary_paths probes only %APPDATA%\npm and %LOCALAPPDATA% — not %ProgramFiles%\nodejs. [Bug] Windows: Codex model discovery times out / empty models (cold start >10s) #2261 calls this out directly ("nvm4w ≠ .nvm"). Your windows_paths.rs was on exactly the right track.
  2. Git Bash probe ordering — main has the WindowsApps alias filtering already (is_windows_apps_alias in git_bash.rs), but your version reorders the probe sequence to prefer the registry lookup over PATH-resolved bash. If that ordering fixed a real failure you hit, it's worth its own PR with the symptom described — I'd take it.

For the record, the CREATE_NO_WINDOW additions here are already covered on main: both spawn sites in buzz-acp/buzz-agent, project_git_exec.rs, and taskkill_tree all set the flag. The console-flash issue is tracked separately in #2685 with #2493 and #2416 open against it, so that work has a home.

@wpfleger96 wpfleger96 closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aleixrodriala @wpfleger96 @tzarebczan